Skip to content

Conversation

@tnull
Copy link
Contributor

@tnull tnull commented Nov 5, 2025

Based on #39.

Blocked on seanmonstar/reqwest#2852.

We drop our (~broken) RetryPolicy, and fully replace it with the new-ish retry policy that is shipping in reqwest since 0.12.23.

tnull added 3 commits November 4, 2025 14:23
While the `RetryPolicy` has a `MaxTotalDelayRetryPolicy`, the retry
`loop` would only check this configured delay once the operation future
actually returns a value. However, without client-side timeouts, we're
not super sure the operation is actually guaranteed to return anything
(even an error, IIUC).

So here, we enable some coarse client-side default timeouts to ensure
the polled futures eventualy return either the response *or* an error we
can handle via our retry logic.
.. as some types are part of our API.
@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Nov 5, 2025

👋 Hi! This PR is now in draft status.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

tnull added 3 commits November 5, 2025 14:16
We here make use of `reqwest`'s `retry` functionlity that was introduced
with its recent v0.12.23 release. As we can't fully replace the old
'application-level' behavior just yet, we configure it to only apply for
error that arent INTERNAL_SERVER_ERROR, which is the status VSS uses to
send error responses.

We set some default values here, but note that these can always can be
overridden by the user when using the `from_client` constructor.
We drop our (~broken) `RetryPolicy`, and replace it with the
new-ish retry policy that is shipping in `reqwest` since 0.12.23.
@tnull tnull force-pushed the 2025-11-drop-custom-retry-policy branch from 9d56321 to 269e1fa Compare November 5, 2025 13:24
@tnull
Copy link
Contributor Author

tnull commented Nov 5, 2025

Nevermind, figured out how to do this already in #39.

@tnull tnull closed this Nov 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants